chore(flags): remove EnableRemovalUnderMaintenanceStep flag #6183
Conversation
kschelonka
force-pushed
the
mntor-5029
branch
from
8cb0c38 to
f031943
Compare
kschelonka
requested review from
Vinnl,
codemist,
mansaj and
rhelmer
and removed request for
codemist
| * License, v. 2.0. If a copy of the MPL was not distributed with this | ||
| * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ | ||
|
|
||
| import { redirect } from "next/navigation"; |
There was a problem hiding this comment.
I believe this page can be removed entirely?
There was a problem hiding this comment.
Everything in dashboard/fix/data-broker-profiles/removal-under-maintenance, including assets/styling.
kschelonka
force-pushed
the
mntor-5029
branch
from
6b1ad7e to
10a0ac6
Compare
| return ( | ||
| <FixView | ||
| subscriberEmails={props.subscriberEmails} | ||
| nextStep={() => moveToNextAvailableStep()} |
There was a problem hiding this comment.
I’m fairly confident that the <FixView> implementation here, along with removing all tests for RemovalUnderMaintenance, is what’s triggering the <100% test coverage regression.
I opened a branch with the changes here for reference which shows 100% coverage.
Previously, we had a type guard on the nextStep prop to account for the navigation logic in RemovalUnderMaintenance, since that step wasn’t route-managed like the rest of the guided resolution flow. If you check our Storybook instances, you’ll notice the nav button is rendered using the first condition (Link) rather than Button (from TelemetryButton), which is unique to this step.
Wanted to flag in case this context helps explain the coverage drop. We definitely should've left more documentation around this type guard.
There was a problem hiding this comment.
Thanks Kaitlyn, I saw the fix here is to remove this behavior then because it's no longer necessary. If that's an incorrect interpretation please let me know.
References:
Jira: MNTOR-5029
Jira: MNTOR-3886
Description
Remove EnableRemovealUnderMaintenanceStep flag
I chose not to make breaking changes to function apis that seemed more general, although args did become unused (e.g. removing the enabledFeatureFlags argument). I can do that if preferred.
I kept the RemovalUnderMaintenance view to avoid refactoring, but it has no logic other than to redirect to eitheredit: deleted this/or/user/dashboarddepending on login state.